-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tracking for Treatment of Undocumented Symbols #33
Conversation
Without getting into the policy claims made by the tests, here are some notes. The files-changed view is a bit overwhelming due to the website parts so putting them here.... At the level jazzy works, there is no difference between struct/class/enum/protocol. I don't think we need repeat all the tests for each: it makes the problem seem more complex than it is and increases the cost of maintaining them. Similarly, the effect of Similarly+, I'm not sure the repeating pair of The The Missing the case that started all this: documented extension of an external undocumented type? The I appreciate the 'u's but I think US spelling is preferred 😉 Some .DS_Store's have been accidentally committed. I do think having these behaviors/tests all in one place is useful (although from my limited experience the collection of real-world projects does exercise enough cases) -- personally I would prefer to see these distilled down to one or two swift files in the misc_jazzy_features project: although that project does not have |
Mostly I’m just proposing a way to track whatever those policy claims end up being. To demonstrate how the method works, I needed to claim something. I am hoping such policy decision eventually get concretely defined by someone, because I can easily imagine people submitting conflicting issues, or fixing one issue overzealously and inadvertently regressing on other corner cases.
That’s why I wanted to do a real demonstration, but because of the following note you made, it didn’t work.
Thanks especially for that note. I was confused as to why your changes in realm/jazzy#825 did not actually register here as changing anything, but I guess that would be why. If the two situations behave differently now (intended or not) maybe both should be tracked?
That is what I did at first, but then I realized it was not self‐documenting when it appears (correctly or incorrectly) in undocumented.json, so I wanted a more descriptive symbol name.
Theoretically, that is in
Yes, classes and enumerations can be removed in favour of structures. I can’t remember if there are any default implementation examples here, but that is one way protocols are already treated differently.
There were so many combinations that to reduce my own confusion, I simply supplied the entire matrix, so I could tell whether or not I had considered everything. Redundancies can be removed where there are no related exceptions.
I saw that, but there is no
I will make some adjustments and see if I can get a demonstration working now that I know about the issue you mentioned that caused it not to work the first time. |
I only just understood what you meant. You are right. Good catch. |
@johnfairh, thanks to your tips, I got it working. This is a demonstration of the diffs this generates for realm/jazzy#825.
|
Doing ⌘F “VISIBLE” on the above mentioned diffs quickly shows that it never occurs in the red, only in the green, meaning nothing is being lost and several fixes are being made. Doing ⌘F “HIDDEN” shows that it never occurs in the green, meaning nothing unwanted is suddenly showing. (Note: You have to consciously ignore |
This was not as quick and easy as I originally thought. It is becoming more work than I really want to invest in something that is just a test. I will close the pull request. I will leave it up to the project administrators to decide whether to delete the related branches ( |
This is the result of discussions in realm/jazzy#819 and realm/jazzy#825.
This proposal creates a sort of manifesto for the desired treatment of undocumented symbols, and in particular the nuances of
--skip-undocumented
andundocumented.json
.It adds a new project to the test suite with symbols in various situations, each named according to the treatment expected, and accompanied by a description of the rationale. It covers many nesting combinations of types, protocols, extensions and methods, each documented, undocumented or with
:nodoc:
. An emphasis has been placed on corner cases and exceptions.Because of the way Jazzy’s tests are set up, whenever any pull request affects this treatment, it will trigger diffs which will bring it to the developer’s attention. Because the intentions are present right there in the triggered diffs, the developer will be able to see either that a currently unsupported edge case was fixed (hurray!), or that a regression occurred (oops!). Because the rationale is also right there in the diffs, the new contributor does not need to ask about it, the veteran contributor does not risk forgetting it, and neither has to hunt through old issues and pull requests to find out why it mattered.
This pull request does not attempt to actually add support for all corner cases immediately.
Neither does this pull request imply that minds cannot be changed or decisions retracted in the future. It merely ensures that the existing reasoning is harder to inadvertently overlook or forget.
These expectations mostly derive from various current and past issues. A few instances, such as the combination of
:nodoc:
withextension
have never been mentioned, so I extrapolated what seemed most consistent with the other expectations.@johnfairh, @jpsim, I would particularly like you two to look over it while the discussion is fresh on your minds in case I forgot something or if there is something you want to change.
I will add a corresponding pull request to Jazzy that assimilates these new test fixtures.